Refactored provider classes from client.ts to providers.ts#5998
Refactored provider classes from client.ts to providers.ts#5998sean-mcmanus merged 30 commits intomicrosoft:masterfrom spacemonkd:provider_5871
Conversation
|
Azure pipelines giving |
|
Hi @devabhishekpal, Thanks for taking a look at this issue. It was assigned to @Colengms as I think he had some ideas as to how he wanted it structured. I will defer to him as to whether this is the direction he had in mind. |
Thank you dear sir. |
I think that error is due to missing the following at the top of provider.ts |
|
Ok sir making changes and committing. |
|
Hi @devabhishekpal . Thanks a lot for jumping in and helping us out. :) The only thing I would have done differently is create a separate file for each provider. We'd also like to move DocumentSymbolProvider, WorkspaceSymbolProvider, FindAllReferencesProvider, and RenameProvider as well. Do you want to tackle these also? :) If not, we could use what is here, and deal with the rest later. |
|
I am on it @Colengms |
|
Could you check up on the variables? @Colengms Not sure if that is desirable so could you kindly check on it. export class AbortRequestIdHolder{ export class ReferenceParamsHolder{ andd export class RenameParamsHolder{ these were originally declared as const values. |
|
Can someone check the updated code and give a review or changes needed? @bobbrow @sean-mcmanus @Colengms @Colengms @michelleangela |
|
@devabhishekpal |
|
@michelleangela thank you for informing. It is alright. As time permits any review of the code would be really helpful, and thanks once again😊 |
|
@devabhishekpal Instead of 'Holder' classes, could you make these individually exported fields for now? We eventually need to reorganize those vars. Also, could you move all providers into a new providers subdirectory? |
|
@devabhishekpal There's a build error on line 1360 of client.ts -- looks like DefaultClient.renamePending should be used. I resolved a merge conflict...not sure if that caused it. |
|
Okayyy I will check it |
Fixed linter errors
|
There are some linter issues: |
|
Yeah I undated those just now @sean-mcmanus sir |
| import { CppSettings } from '../settings'; | ||
| import * as editorConfig from 'editorconfig'; | ||
|
|
||
|
|
There was a problem hiding this comment.
There's a double newline here and in a few other files. If you feel like it, you could change the existing "no-multiple-empty-lines" in .eslintrc.js to be "no-multiple-empty-lines": ["error", { "max": 1, "maxEOF": 1, "maxBOF": 0}].
If not, we could do that in a separate PR.
|
FYI, we're going to delay checking this in until we can merge to release for 0.30.0-insiders5 and 1.0.0...the merge might be in a few hours though...or maybe tomorrow. |
|
Okayy |
Updated .eslintrc.js
|
I changed the eslint rules. I will fix the multiple spaces on other files by evening |
|
Sorry about so many commits...I was editing from my phone so I had to edit each file individually... I have fixed the errors....Kindly check the same.. @sean-mcmanus sir |
Cool, thanks. We might be able to check this in tomorrow after we merge for 1.0 (not sure if @Colengms had any more review feedback). |
|
Okayy |
|
Is there any other issue for me to look at? @sean-mcmanus sir |
No issues. We can probably check it in Monday. We've been busy preparing our 1.0 release. |
|
Okayy sir |
Refactored Provider classes ( FoldingRangeProvider, SemanticTokensProvider, DocumentFormattingEditProvider, DocumentRangeFormattingEditProvider, OnTypeFormattingEditProvider ) from client.ts to providers.ts.
Pull request for issue: #5871
Kindly review and suggest necessary changes @bobbrow @Colengms @michelleangela